-
Notifications
You must be signed in to change notification settings - Fork 961
Support for DECODE operator #3216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@tensorflow/micro Add support for alternate decompression memory to DECODE operator. Additional unit tests. Update generic benchmark application and Makefile. bug=fixes tensorflow#3212
@tensorflow/micro Add support for application custom DECODE operator registrations. Additional unit tests. bug=fixes tensorflow#3215
veblush
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
| // Set the custom DECODE operator registrations. | ||
| // Can only be called during the kInit state. | ||
| virtual TfLiteStatus SetCustomDecodeRegistrations( | ||
| const std::initializer_list<CustomDecodeRegistration>& registrations) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SetCustomDecodeRegistrations could pose a lifetime safety issue. Storing a pointer to a std::initializer_list is dangerous because the underlying array is often a temporary object that gets destroyed immediately after the function returns. Please update the API to accept a pointer to a persistent array and a size count (e.g., const CustomDecodeRegistration* registrations, size_t size) so the caller is explicitly responsible for ensuring the data's lifetime matches that of the interpreter. (also its comment should mention about the lifetime expectation)
With this change, this kind of help function would be helpful .
template <size_t N>
TfLiteStatus SetCustomDecodeRegistrations(const CustomDecodeRegistration (®s)[N]) {
return SetCustomDecodeRegistrations(regs, N);
}
We might want to make a similar change to decompress_regions_ as well but it could be a separate PR.
| uint8_t reserved1; // reserved | ||
| uint8_t reserved2; // reserved | ||
| uint8_t reserved3; // reserved | ||
| tflite::DecodeState* (*func)(const TfLiteContext*, MicroProfilerInterface*); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of func, factory or create_state would be better to describes what the function will do.
@tensorflow/micro
Add support for application custom DECODE operator registrations.
Additional unit tests.
bug=fixes #3215